Skip to content

emit unstable_feature from build script #69

Merged
MusicalNinjaDad merged 6 commits into
mainfrom
emit_unstable_feature
Apr 14, 2026
Merged

emit unstable_feature from build script #69
MusicalNinjaDad merged 6 commits into
mainfrom
emit_unstable_feature

Conversation

@MusicalNinjaDad
Copy link
Copy Markdown
Owner

improvements based upon discussion in /cuviper/autocfg#81

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="build.rs" line_range="23-27" />
<code_context>
-    use std::assert_matches;
+/// Location of assert_matches!() macro. Stabilisation was reverted at last minute
+/// on 2026-04-10, leaving the macro in the new planned location.
+enum AssertMatchesLocation {
+    /// Macro is at `std::assert_matches`
+    Root,
+    /// Macro is at `std::assert_matches::assert_matches`
+    Module,
+}

</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid duplicating the `assert_matches_*` cfg key strings in multiple places.

The cfg values `"assert_matches_in_root"` and `"assert_matches_in_module"` are currently hard-coded in both `Display` and `emit_possibilities`, so a change in one place could silently desync the mapping between `emit_possibility` and `emit`. Please centralize these as `const` values or via an `AssertMatchesLocation` method that returns the cfg key, and use that in both locations.

Suggested implementation:

```rust
}

/// Location of assert_matches!() macro. Stabilisation was reverted at last minute
/// on 2026-04-10, leaving the macro in the new planned location.
enum AssertMatchesLocation {
    /// Macro is at `std::assert_matches`
    Root,
    /// Macro is at `std::assert_matches::assert_matches`
    Module,
}

impl AssertMatchesLocation {
    /// Returns the corresponding `cfg` key for this location.
    ///
    /// Centralizing these values here avoids having the string literals duplicated
    /// in multiple places (e.g. `Display` and `emit_possibilities`).
    const fn cfg_key(&self) -> &'static str {
        match self {
            AssertMatchesLocation::Root => "assert_matches_in_root",
            AssertMatchesLocation::Module => "assert_matches_in_module",
        }
    }
}

use autocfg::AutoCfg;

```

To fully implement your suggestion, any places that currently hard-code the cfg key strings must be updated to use the centralized source:

1. In the `impl Display for AssertMatchesLocation`:
   - Replace occurrences of `"assert_matches_in_root"` and `"assert_matches_in_module"` with `self.cfg_key()` (or with `AssertMatchesLocation::Root.cfg_key()` / `AssertMatchesLocation::Module.cfg_key()` if it’s not a method on `self` in that context).

2. In `emit_possibilities` (or similar helper that emits the cfg flags):
   - Where it currently does something like `println!("cargo:rustc-cfg=assert_matches_in_root")` or `..._in_module`, replace those string literals with the relevant `AssertMatchesLocation` variant’s `cfg_key()`:
     - `AssertMatchesLocation::Root.cfg_key()`
     - `AssertMatchesLocation::Module.cfg_key()`

3. If there are any other uses of these cfg strings elsewhere in `build.rs`, update them to also call `cfg_key()` on the appropriate `AssertMatchesLocation` rather than using raw string literals.

These changes will ensure that any future edits to the cfg names are made in a single location (`cfg_key`) and automatically applied everywhere else.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread build.rs
@MusicalNinjaDad MusicalNinjaDad enabled auto-merge (squash) April 14, 2026 07:40
@MusicalNinjaDad MusicalNinjaDad merged commit 3ae814d into main Apr 14, 2026
18 checks passed
@MusicalNinjaDad MusicalNinjaDad deleted the emit_unstable_feature branch April 14, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant